Multiple fixes: buffer bound checks#721
Merged
rizlik merged 20 commits intowolfSSL:masterfrom Mar 12, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens several code paths against out-of-bounds reads/writes and secret leakage, and adds/extends unit tests to lock the behavior in.
Changes:
- Add bound/capacity checks for TPM NV blob reads, unseal output size, and encrypted disk header TLVs.
- Zeroize sensitive buffers across more failure paths (disk encryption, TPM unseal, key hashing).
- Improve tests and build matrix to cover the new checks and configurations (no partitions, SHA-384, SHA3-384, TPM blob, update-disk).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/unit-tests/unit-update-disk.c | New unit tests for update-disk encryption failure/boot paths and TLV truncation handling |
| tools/unit-tests/unit-tpm-rsa-exp.c | Add test ensuring RoT digest comparison avoids memcmp (constant-time compare) |
| tools/unit-tests/unit-tpm-blob.c | New unit tests for TPM blob NV bounds/auth checks and unseal capacity handling |
| tools/unit-tests/unit-string.c | Extend tests for strncpy padding / termination behavior |
| tools/unit-tests/unit-nvm.c | Add test ensuring flash-write error stops partition-magic update |
| tools/unit-tests/unit-mock-flash.c | Add injectable failure path for hal_flash_write() in unit tests |
| tools/unit-tests/unit-image.c | Expand hashing/config coverage; add RAM-boot size cap test without partitions |
| tools/unit-tests/Makefile | Add new test targets (update-disk, TPM blob, no partitions, SHA-384, SHA3-384) |
| src/xmalloc.c | Fix xmalloc pool sizing for small-stack / SPMATH configurations |
| src/x86/ahci.c | Initialize unseal secret-size parameter correctly before use |
| src/update_flash.c | Initialize unseal secret-size parameters correctly before use |
| src/update_disk.c | Add TLV bounds check and force-zero disk encryption key/nonce across failure + pre-boot |
| src/tpm.c | Add auth-size caps, NV blob size bounds checks, unseal capacity checks, constant-time digest compare |
| src/string.c | Avoid repeated strlen in strcat; add zero-padding behavior to strncpy |
| src/libwolfboot.c | Propagate hal_flash_write() failure from partition_magic_write() |
| src/image.c | Zero hash buffers on early return; cap fw_size in RAM-boot mode (no partitions) |
| include/target.h.in | Make RAM-boot max-size define optional via generated define block |
| hal/stm32h7.c | Adjust bank argument usage in flash wait/unlock/lock flow |
| docs/TPM.md | Document wolfBoot_unseal_auth() secret_sz as in/out capacity/result |
| Makefile | Generate optional WOLFBOOT_RAMBOOT_MAX_SIZE define in target.h |
| .github/workflows/test-library.yml | Remove matrix exclusions now that the underlying issue is fixed |
Comments suppressed due to low confidence (1)
src/string.c:180
- The PR description says
strncpynow enforces null-termination, but this implementation only zero-pads after encountering a\\0insrc—it still does not guarantee a terminator when truncating (srclength >=n), which matches standardstrncpy. Either update the PR description to reflect the actual change (zero-padding behavior) or intentionally enforce termination (with clear documentation of the nonstandard semantics).
char *strncpy(char *dst, const char *src, size_t n)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
The caller is now required to pass the available space in buffer. The actual size of the unsealed secret is returned. F/442
+ Re-enabled old faulty tests F/366
rizlik
requested changes
Mar 12, 2026
danielinux
force-pushed
the
buffer-hardening
branch
from
df06a6e to
87a4ffc
Compare
danielinux
commented
Mar 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/update_disk.c:132
get_decrypted_blob_version()still dereferencespasuint16_t*before verifying thatpis aligned / not a padding byte. Because the loop can incrementpby 1, the next iteration can execute*((uint16_t*)p)on an odd address, which is undefined behavior and can fault on alignment-sensitive targets. Fix by checking for padding/unaligned addresses before any halfword/word loads (or by using byte-wise loads / memcpy-based LE decoding).
tlv_type = *((uint16_t*)p);
tlv_len = *((uint16_t*)(p + 2));
if (tlv_type == 0 || tlv_type == 0xFFFF)
break;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
rizlik
approved these changes
Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
83091e5 TPM: RoT comparison made Constant time F/448
1b54481 TPM NV blob functions: Limit authsz to buffer capacity F/375
73a10e5 Added bound check to get_decrypted_blob_version F/374
927b8c9 Force-zero secrets in update_disk.c F/97
ff20310 strings.c: enforce null-termination on strncpy short src, optimize strcat F/439 and F/440
45eaf6f Propagate hal_flash_write() error when writing partition magic F/437
9194129 key_sha384: zero hash buffer to cover for early error F/370
3153775 Fix xmalloc bucket size issue with "SPMATH=1 WOLFBOOT_SMALL_STACK=1" F/366
d3ff22e wolfBoot_unseal_blob: ensure secret is zeroed from stack after use F/444
3e07cbe wolfBoot_unseal_auth(): secretSz is now in/out param F/442
0365eb7 Check fw_size when WOLFBOOT_FIXED_PARTITIONS is off F/363
68a1b01 Validate blob size from TPM NV storage F/372
45749ed STM32H7: Fix wrong block size in flash ops F/367
df06a6e xmalloc: Add extra pools to fix SPMATHALL=1 WOLFBOOT_SMALL_STACK=1 builds